-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: video upload issue through image picker #2204
Conversation
// Since Expo MediaLibrary doesn't return the mimetype of the image/video, we have to derive the mimeType. | ||
const mimeType = lookup(asset.filename) || 'multipart/form-data'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if expo is the only that doesnt return mime type why is it being done for both here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per expo docs, mimetype is optionally present in the asset type
https://docs.expo.dev/versions/latest/sdk/document-picker/#documentpickerasset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also present in older versions
https://docs.expo.dev/versions/v47.0.0/sdk/document-picker/#documentresult
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you read the comment above, I am not saying expo doesn't do that(send mimeType in response). Expo MediaLibrary
doesn't do that, Expo DocumentPicker
does. In our SDK we don't do a lookup for the code for the document picker or file picker, we do it for the image picker/media library @santhoshvai
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought its related to document picker, because you said that this was broken by the docu,ent picker version update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, what I meant was the previous PR of the document picker that was merged yesterday caused this issue of video upload because mime-type was compulsory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a better way to fix this and that we discussed yesterday, handling lookup in the expo-package and native-package and sending mimetype through the package itself
Kudos, SonarCloud Quality Gate passed! 0 Bugs 0.0% Coverage The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
🎉 This PR is included in version 5.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎯 Goal
🛠 Implementation details
🎨 UI Changes
iOS
Android
🧪 Testing
☑️ Checklist
develop
branch